-
Notifications
You must be signed in to change notification settings - Fork 6.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Increase bsim tests watchdog deadline - again #80916
base: main
Are you sure you want to change the base?
Conversation
Fix spurious ISO Sync Receiver stall due to uninitialised value accessed due to regression introduced by commit 64facee ("Bluetooth: controller: Stop Sync ISO ticker when establishment fails"). Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
These tests have been seen failing in CI due to the real time execution timeout. Let's increase it so it does not happen. Signed-off-by: Alberto Escolar Piedras <[email protected]>
Increase the EXECUTION_TIMEOUT (real time timeout at which the test will be killed), so we have more margin for CI. Signed-off-by: Alberto Escolar Piedras <[email protected]>
Increase the EXECUTION_TIMEOUT (real time timeout at which the test will be killed), so we have more margin for CI. Signed-off-by: Alberto Escolar Piedras <[email protected]>
Increase the EXECUTION_TIMEOUT (real time timeout at which the test will be killed), so we have more margin for CI. Signed-off-by: Alberto Escolar Piedras <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is our CI just getting slower, or is Zephyr/BSIM getting slower? :D
We discussed it earlier where I suggest to increase the default timeout. At this point a default of 300 seconds is starting to look better. We already have increased the timeout for more than a hundred tests, and this is increasing them even further.
Don't know. One of the 3 tests that was timing out is newish, the other 2 got realencryption enabled recently (which adds a bit of overhead). Zephyr is not getting leaner. And CI nodes seem to be quite variable in performance. bsim itself, specially for the 52 should be quite unchanged for the last few months. So I'd blame bloat in your code :p |
:( Jokes aside, we are adding more and more testing and more criteria, which of course also increases the runtime, but shouldn't affect them this much. Would be interesting to have some timing benchmarks on some audio tests so we could track that |
It would be. Out of curiosity I just checked for tests/bsim/bluetooth/audio/test_scripts/bap_unicast_audio.sh, the server in 3.6.0 and as it is now in main. The difference is relatively small mainTotal inst: 753,523,067 3.6.0Total inst: 714,650,574 There is lots of other minor differences, but it is difficult to see a clear pattern, some things use more time, some less. |
One thing though, we are still running in CI with coverage enabled, but I don't think we really do much with that data for the last couple of years. That takes a significant chunk of time. |
I think there was some effort a while ago to actually publish this, so it would be part of https://app.codecov.io/gh/zephyrproject-rtos/zephyr or at least accessible which I don't think it really is anywhere (and not very visible). #56421 |
Very much like it was done in #70750
Increase tests timeouts to ensure they do not trigger in CI.
A test timeout has been increased if the time it took in one recent CI run was > 1/3 of the timeout (or what it took in my own machine > 1/30 of the timeout)
If anyhow, I was increasing the timing I have increased it by quite a bit, so we are far from the 3x limit.
(Note the default timeout is 30 seconds)
Sits on top of
(Those 2 commits are already in main, so they can ignored in this PR. Merging this PR will drop those 2 commits)
This PR is not urgent, as the urgent timeout increases should be in #80896, but all changes are trivial and harmless enough.